Skip to content

[SPARK-6975][Yarn] Fix argument validation error#5551

Closed
jerryshao wants to merge 2 commits intoapache:masterfrom
jerryshao:SPARK-6975
Closed

[SPARK-6975][Yarn] Fix argument validation error#5551
jerryshao wants to merge 2 commits intoapache:masterfrom
jerryshao:SPARK-6975

Conversation

@jerryshao
Copy link
Contributor

@jerryshao jerryshao commented Apr 17, 2015

numExecutors checking is failed when dynamic allocation is enabled with default configuration. Details can be seen is SPARK-6975. @sryza, please help me to review this, not sure is this the correct way, I think previous you change this part :)

srowen
srowen reviewed Apr 17, 2015
View reviewed changes
yarn/src/main/scala/org/apache/spark/deploy/yarn/ClientArguments.scala Outdated
Copy link
Member

@srowen srowen Apr 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enabled -> enabled. I think this is simpler to state as "Number of executors was $numExecutors, but must be at least 1 (or 0 if dynamic executor allocation is enabled)."

Copy link
Contributor Author

@jerryshao jerryshao Apr 17, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will change the statement.

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #30466 has finished for PR 5551 at commit 77bdcbd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #30471 has finished for PR 5551 at commit 4335da1.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@srowen
Copy link
Member

srowen commented Apr 17, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #30481 has finished for PR 5551 at commit 4335da1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@vanzin
Copy link
Contributor

vanzin commented Apr 17, 2015

Looks fine to me. Seems like it might be useful (and easy) to have different messages for each case, but not a big deal.

@andrewor14
Copy link
Contributor

andrewor14 commented Apr 18, 2015

This looks fine. The previous setting was very confusing to the user because they would have run into this by following the docs. Thanks for fixing this I'm merging this into master and 1.3

asfgit pushed a commit that referenced this pull request Apr 18, 2015
@jerryshao
[SPARK-6975][Yarn] Fix argument validation error
7d0ab55 
`numExecutors` checking is failed when dynamic allocation is enabled with default configuration. Details can be seen is [SPARK-6975](https://issues.apache.org/jira/browse/SPARK-6975). sryza, please help me to review this, not sure is this the correct way, I think previous you change this part :)

Author: jerryshao <saisai.shao@intel.com>

Closes #5551 from jerryshao/SPARK-6975 and squashes the following commits:

4335da1 [jerryshao] Change according to the comments
77bdcbd [jerryshao] Fix argument validation error

(cherry picked from commit d850b4b)
Signed-off-by: Andrew Or <andrew@databricks.com>
@asfgit asfgit closed this in d850b4b Apr 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jerryshao @SparkQA @srowen @vanzin @andrewor14